Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[fix] #4267: Introduce p2p idle timeout #4398

Merged
merged 1 commit into from
Apr 5, 2024

Conversation

Erigara
Copy link
Contributor

@Erigara Erigara commented Mar 29, 2024

Description

Previously iroha tried to connect indefinitely to idle peer.

Introduce idle timeout for p2p communication:

Each peer track the last time it received message (or ping) from particular peer, if this time is larger than timeout peer is disconnected.

If no other messages were produced in time equal to half timeout ping is send to prove that peer isn't idle.

Linked issue

Closes #4267

How to test

  1. setup iroha network with multiple peers using LOGLEVEL=TRACE ./scripts/test_env.py setup
  2. kill one of the peers (e.g. iroha1)
  3. start listening on p2p port this peer was using nc -l 1338
  4. check that peers terminate connection with this "peer" after timeout

@Erigara Erigara added Bug Something isn't working iroha2-dev The re-implementation of a BFT hyperledger in RUST labels Mar 29, 2024
@Erigara Erigara self-assigned this Mar 29, 2024
@Erigara
Copy link
Contributor Author

Erigara commented Mar 29, 2024

Waiting for @BAStos525 conformation that issue is resolved to undraft PR.

@Erigara
Copy link
Contributor Author

Erigara commented Mar 29, 2024

Got confirmation that issue is resolved.

@Erigara Erigara marked this pull request as ready for review March 29, 2024 17:33
config/src/parameters/actual.rs Outdated Show resolved Hide resolved
p2p/src/peer.rs Outdated Show resolved Hide resolved
p2p/src/peer.rs Outdated Show resolved Hide resolved
p2p/src/peer.rs Outdated Show resolved Hide resolved
@DCNick3 DCNick3 self-assigned this Apr 2, 2024
@Erigara Erigara force-pushed the connected_peers branch 2 times, most recently from 433ae50 to 2499098 Compare April 3, 2024 10:29
DCNick3
DCNick3 previously approved these changes Apr 3, 2024
@DCNick3
Copy link
Contributor

DCNick3 commented Apr 3, 2024

Need to update config snapshots it seems

@github-actions github-actions bot added the config-changes Changes in configuration and start up of the Iroha label Apr 3, 2024
Copy link

github-actions bot commented Apr 3, 2024

@BAStos525

Copy link
Contributor

@0x009922 0x009922 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please, while changing configuration, also contribute into the reference:

hyperledger-iroha/iroha-2-docs#397

config/src/parameters/actual.rs Outdated Show resolved Hide resolved
config/src/parameters/actual.rs Outdated Show resolved Hide resolved
0x009922
0x009922 previously approved these changes Apr 4, 2024
@Erigara Erigara merged commit 3772c56 into hyperledger-iroha:iroha2-dev Apr 5, 2024
9 of 11 checks passed
@timofeevmd timofeevmd added the QA-confirmed This bug is reproduced and needs a fix label Apr 18, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something isn't working config-changes Changes in configuration and start up of the Iroha iroha2-dev The re-implementation of a BFT hyperledger in RUST QA-confirmed This bug is reproduced and needs a fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants